Skip to content

Verify TLS certificate before downloading binaries#491

Closed
philandstuff wants to merge 1 commit intoNixOS:masterfrom
philandstuff:securely-download-binaries
Closed

Verify TLS certificate before downloading binaries#491
philandstuff wants to merge 1 commit intoNixOS:masterfrom
philandstuff:securely-download-binaries

Conversation

@philandstuff
Copy link
Contributor

The --insecure flag to curl tells curl not to bother checking if the TLS
certificate presented by the server actually matches the hostname
requested, and actually is issued by a trusted CA chain. This almost
entirely negates any benefit from using TLS in the first place.

This removes the --insecure flag to ensure we actually have a secure
connection to the intended hostname before downloading binaries.

Manually tested locally within a dev-shell; was able to download
binaries from https://cache.nixos.org without issue.

@vcunat
Copy link
Member

vcunat commented Mar 4, 2015

We do this because we check the hash of the result instead of securing connections in any way.

@philandstuff
Copy link
Contributor Author

@vcunat oh. Is this documented somewhere? I see there's a comment in download-using-manifests.pl.in but none in download-from-binary-cache.pl.in.

Why does cache.nixos.org bother with https at all if you don't need TLS to guarantee integrity? Also, do you care about confidentiality at all or is that not a concern? TLS can give confidentiality in a way that hash checking can't, but only if you verify certificates.

I'm trying to be helpful but I don't understand the rationale for the use of https here.

@vcunat
Copy link
Member

vcunat commented Mar 4, 2015

Ah, well, the *.narinfo files from the binary cache contain hash of the result. Maybe that one's downloaded insecurely as well, but for me personally it's more important that *.narinfo can contain signatures of builders certifying the hash (although we don't have them on cache.nixos.org yet; see #75).

@philandstuff
Copy link
Contributor Author

Thank you very much, that's interesting and useful context.

I agree that it's more important to verify *.narinfo signatures. That is definitely the correct priority.

However, I still don't understand: why would you bother using https at all if you don't check the cert? Especially since nix creates a new connection for each call to download-from-binary-cache.pl, that's a lot of extra overhead for TLS handshakes, particularly if doing a big system update (eg from fresh AMI to latest nixos release).

@edolstra
Copy link
Member

edolstra commented Mar 4, 2015

We use https to ensure the integrity of the .narinfo files (where we do check the certificate). Getting the corresponding .nars over https is just a side-effect of using the same URL prefix.

However, I'm fine with removing the --insecure flag because it's pointless (since if we can verify the certificate while fetching .narinfo file, we can do the same while fetching .nar file).

@philandstuff
Copy link
Contributor Author

should I also add the use of --insecure in download-using-manifests.pl to this PR? Also what about in corepkgs/fetchurl.nix (or is that outside the scope of this PR)?

@edolstra
Copy link
Member

edolstra commented Mar 4, 2015

Yes for download-using-manifests, but not for fetchurl (since it doesn't have a certificate bundle).

The --insecure flag to curl tells curl not to bother checking if the TLS
certificate presented by the server actually matches the hostname
requested, and actually is issued by a trusted CA chain.  This almost
entirely negates any benefit from using TLS in the first place.

This removes the --insecure flag to ensure we actually have a secure
connection to the intended hostname before downloading binaries.

Manually tested locally within a dev-shell; was able to download
binaries from https://cache.nixos.org without issue.
@philandstuff philandstuff force-pushed the securely-download-binaries branch from efa18a3 to 683ab27 Compare March 4, 2015 21:53
@philandstuff
Copy link
Contributor Author

@edolstra updated to remove --insecure from download-using-manifests

@philandstuff
Copy link
Contributor Author

argh I think this could do with testing on a pure nixos system; I only tested on ubuntu and I think it may have been using the ubuntu system ca list. I'm not sure how CAs work on nixos so I don't know if this will work as expected -- when I tried with ./dev-shell --pure it couldn't find a ca list to verify the certs...

@edolstra
Copy link
Member

edolstra commented Mar 4, 2015

You need to set $OPENSSL_X509_CERT_FILE to /etc/ssl/certs/ca-bundle.crt.

@wkennington
Copy link
Contributor

Note that it is no longer OPENSSL_X509_CERT_FILE, use SSL_CERT_FILE instead.

@edolstra
Copy link
Member

edolstra commented Jan 5, 2016

Thanks, applied!

@edolstra edolstra closed this Jan 5, 2016
@copumpkin
Copy link
Member

👍

zolodev pushed a commit to zolodev/nix that referenced this pull request Jan 1, 2024
zolodev pushed a commit to zolodev/nix that referenced this pull request Jan 1, 2024
NIX_PATH includes nixpkgs in cross-compilation.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants